Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(helm): Fixed migrate not being consistent in name for hooks #4968

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

elliotcourant
Copy link

What this PR does

This makes it so that the yaml being created for the migrate job is consistent when useHook is true or false. At the moment if use hook is false, the the name on the yaml and in the metadata match. But if use hook is true then the name on the object itself is different from the one in the metadata.

Which issue(s) this PR closes

Didn't open an issue for this since it's minor.

Basically I have an "infra" repo that generates the yaml from several helm charts and version controls the generated yaml. It's an easy way to make sure that a chart update doesn't absolutely clobber something and makes it very easy to see whats actually getting pushed out to the kube cluster before actually applying it.

Because of the way this "metadata.name" field was being generated, it would result in the file being changed each time the yaml for the repository was generated which made for some odd diffs when something completely different may have changed.

Checklist

  • Unit, integration, and e2e (if applicable) tests updated
  • Documentation added (or pr:no public docs PR label added if not required)
  • Added the relevant release notes label (see labels prefixed w/ release:). These labels dictate how your PR will
    show up in the autogenerated release notes.

Not aware of any tests for this specifically in this repo that I can see, or documentation around this specific behavior.

@elliotcourant elliotcourant requested a review from a team August 30, 2024 22:45
@CLAassistant
Copy link

CLAassistant commented Aug 30, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the pr:stale Added to a PR that has been deemed "stale". Managed by the actions/stale GitHub Action label Sep 30, 2024
@elliotcourant
Copy link
Author

Just looking for feedback still, not stale

@github-actions github-actions bot removed the pr:stale Added to a PR that has been deemed "stale". Managed by the actions/stale GitHub Action label Oct 1, 2024
This makes it so that the yaml being created for the migrate job is
consistent when `useHook` is `true` or `false`. At the moment if use
hook is false, the the name on the yaml and in the metadata match. But
if use hook is true then the name on the object itself is different from
the one in the metadata.
@elliotcourant elliotcourant requested a review from a team as a code owner November 9, 2024 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants